Skip to content

feat(experiment): add apps.icon.set support for non-hosted app icons#469

Open
srtaalej wants to merge 10 commits intomainfrom
ale-set-icon
Open

feat(experiment): add apps.icon.set support for non-hosted app icons#469
srtaalej wants to merge 10 commits intomainfrom
ale-set-icon

Conversation

@srtaalej
Copy link
Copy Markdown
Contributor

@srtaalej srtaalej commented Apr 3, 2026 •
edited
Loading

Changelog

No user-facing changes until apps.icon.set endpoint is live

Summary

This PR adds IconSet() API method targeting the apps.icon.set endpoint for non-hosted (Bolt) app icons. Gated behind the set-icon experiment flag (--experiment=set-icon) (see Note)

Note

🔴 The apps.icon.set endpoint is not yet publicly available. This PR is ready to ship once the endpoint is live. No changes were made to the existing apps.hosted.icon flow.

Required scope (app_configurations:write) may be subject to change before the endpoint goes public

Test plan

slack run --experiment=set-icon with an icon.png uploads icon once endpoint is available
slack run without the experiment flag does not attempt icon upload for non-hosted apps
slack deploy for hosted apps continues to work as before

Requirements

@srtaalej srtaalej self-assigned this Apr 3, 2026
@srtaalej srtaalej added enhancement M-T: A feature request for new functionality experiment Experimental feature accessed behind the --experiment flag or toggle semver:patch Use on pull requests to describe the release version increment labels Apr 3, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 30.00000% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.17%. Comparing base (ca16a0f) to head (b382d68).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/pkg/apps/install.go 0.00% 13 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #469      +/-   ##
==========================================
- Coverage   71.19%   71.17%   -0.02%     
==========================================
  Files         222      222              
  Lines       18595    18612      +17     
==========================================
+ Hits        13238    13248      +10     
- Misses       4178     4187       +9     
+ Partials     1179     1177       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@srtaalej srtaalej marked this pull request as ready for review April 6, 2026 22:56
@srtaalej srtaalej requested a review from a team as a code owner April 6, 2026 22:56
@srtaalej srtaalej requested review from mwbrooks and zimeg April 7, 2026 17:07
zimeg
zimeg reviewed Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐣 A few quick comments and questions and I'm excited to test this next!

@srtaalej srtaalej requested a review from zimeg April 7, 2026 19:41
zimeg
zimeg requested changes Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srtaalej Right now I'm not having luck with this when testing in a shared team sandbox and the error isn't so obvious to me...

Let me know if this is something perhaps with the image I use? I'll include it here for testing and other reviewers:

Image

internal/api/icon.go
const (
appIconMethod = "apps.hosted.icon"
// appIconSetMethod is the API method for setting app icons for non-hosted apps.
appIconSetMethod = "apps.icon.set"
Copy link
Copy Markdown
Member

@zimeg zimeg Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔬 issue: This outputs a warning to verbose logs and it's not clear what I might not be including?

uploading icon
icon error: https://slack.com/api/apps.icon.set error: missing_arguments
   Error updating app icon: https://slack.com/api/apps.icon.set error: missing_arguments
   Finished in 0.7s

Copy link
Copy Markdown
Contributor Author

@srtaalej srtaalej Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you uploading through the manifest or is the image just in the root of the project?

Copy link
Copy Markdown
Member

@zimeg zimeg Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srtaalej I saved this file to the root of the project 🌲 ✨

Copy link
Copy Markdown
Contributor Author

@srtaalej srtaalej Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the "file" fielf for the api request had the wrong name! can you try again ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zimeg zimeg zimeg requested changes

@mwbrooks mwbrooks Awaiting requested review from mwbrooks

Requested changes must be addressed to merge this pull request.

Assignees

@srtaalej srtaalej

Labels

enhancement M-T: A feature request for new functionality experiment Experimental feature accessed behind the --experiment flag or toggle semver:patch Use on pull requests to describe the release version increment

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@srtaalej @zimeg